Skip to content

Improve IPv6 address validation logic#79

Merged
HackingRepo merged 1 commit intomainfrom
HackingRepo-patch-3
Apr 27, 2026
Merged

Improve IPv6 address validation logic#79
HackingRepo merged 1 commit intomainfrom
HackingRepo-patch-3

Conversation

@HackingRepo
Copy link
Copy Markdown
Owner

Enhance IPv6 address validation by adding checks for IPv4-mapped and IPv4-compatible addresses, along with additional manual blocks for specific IPv6 ranges.
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Enhance IPv6 and URL hostname validation security

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Enhanced IPv6 address validation to detect IPv4-mapped and IPv4-compatible addresses
• Added manual blocks for specific IPv6 ranges (64:ff9b:1::/48, 5f00::/8, 3fff::/20, fec0::/10)
• Fixed hostname parsing in URL validation to strip IPv6 bracket notation
• Improved internal IP detection logic for security vulnerability mitigation
Diagram
flowchart LR
  A["IPv6 Validation"] --> B["Check IPv4-mapped/compatible"]
  B --> C["Recursively validate as IPv4"]
  A --> D["Check extra bad ranges"]
  D --> E["Match against CIDR blocks"]
  F["URL Hostname Parsing"] --> G["Strip IPv6 brackets"]
  G --> H["Validate cleaned hostname"]
Loading

Grey Divider

File Changes

1. dist/helpers.js 🐞 Bug fix +28/-5

IPv6 validation and URL hostname parsing improvements

• Enhanced is_ip_internal() function to handle IPv4-mapped (ipv4Mapped) and IPv4-compatible
 (rfc6145) IPv6 addresses by recursively validating their IPv4 representation
• Added manual CIDR range blocks for specific IPv6 ranges (64:ff9b:1::/48, 5f00::/8, 3fff::/20,
 fec0::/10) to catch additional internal/reserved addresses
• Fixed hostname extraction in is_url_safe() and is_url_safe_debug() functions to strip IPv6
 bracket notation using regex replacement
• Improved security by preventing bypass of internal IP checks through IPv6 address variations

dist/helpers.js


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Apr 27, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0)

Grey Divider


Action required

1. Dist/source logic diverges 🐞 Bug ⛨ Security
Description
The IPv6/URL hostname hardening was applied only in dist/helpers.js, while src/helpers.ts still
contains the old logic; running tsc (the project build) will overwrite dist and drop the fix. Code
paths importing from src/ (e.g., benches) will also continue using the old behavior, undermining
the intended GHSA fix.
Code

dist/helpers.js[R224-249]

+        if (range !== "unicast") {
+            // Check for IPv4-mapped or IPv4-compatible addresses
+            if (range === "ipv4Mapped" || range === "rfc6145") {
+                try {
+                    // @ts-ignore
+                    const ipv4 = parsed.toIPv4Address();
+                    return is_ip_internal(ipv4.toString());
+                }
+                catch {
+                    return true;
+                }
+            }
+            return true;
+        }
+        // Additional manual blocks for IPv6
+        const extraBadRanges = [
+            ipaddr.parseCIDR("64:ff9b:1::/48"),
+            ipaddr.parseCIDR("5f00::/8"),
+            ipaddr.parseCIDR("3fff::/20"),
+            ipaddr.parseCIDR("fec0::/10"),
+        ];
+        for (const [range, bits] of extraBadRanges) {
+            // @ts-ignore
+            if (parsed.match(range, bits))
+                return true;
+        }
Evidence
tsconfig.json compiles only src/ into dist/, and package.json’s build script runs tsc, so
dist is derived from src and will be regenerated from the unchanged source. The source still uses
the old IPv6 internal check and does not strip brackets from parsed.hostname, while benches import
from ../src/utils and therefore exercise the unfixed src implementation.

tsconfig.json[2-12]
package.json[21-36]
src/helpers.ts[224-243]
src/helpers.ts[482-510]
dist/helpers.js[215-252]
dist/helpers.js[425-450]
benches/ssrf-validation.bench.ts[2-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Security-related IPv6/hostname validation changes were made directly in the generated file `dist/helpers.js` only. The authoritative source `src/helpers.ts` still has the previous behavior, so `npm run build` (`tsc`) will overwrite the fix and internal repo imports from `src/` will remain vulnerable.

### Issue Context
- `tsconfig.json` outputs to `dist` and includes only `src`.
- `package.json` build script is `tsc`.
- Benchmarks import from `../src/utils`.

### Fix Focus Areas
- src/helpers.ts[224-243]
- src/helpers.ts[482-520]
- dist/helpers.js[215-252]
- dist/helpers.js[425-460]

### What to do
1. Implement the same IPv6 validation logic (including IPv4-mapped handling + extra blocked ranges) in `src/helpers.ts:is_ip_internal`.
2. Apply the same hostname normalization (`parsed.hostname.replace(...)`) in `src/helpers.ts:is_url_safe` (and debug variant if intended).
3. Re-run `npm run build` to regenerate `dist/` from `src/` and commit the generated output so `dist/` matches `src/`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

2. CIDR parsed per call 🐞 Bug ➹ Performance
Description
is_ip_internal() parses constant IPv6 CIDRs on every invocation by recreating extraBadRanges,
causing avoidable repeated work. While likely not a bottleneck versus DNS/network I/O, this is a
simple optimization and reduces per-call overhead in loops that classify many IPs.
Code

dist/helpers.js[R238-244]

+        // Additional manual blocks for IPv6
+        const extraBadRanges = [
+            ipaddr.parseCIDR("64:ff9b:1::/48"),
+            ipaddr.parseCIDR("5f00::/8"),
+            ipaddr.parseCIDR("3fff::/20"),
+            ipaddr.parseCIDR("fec0::/10"),
+        ];
Evidence
extraBadRanges is created inside is_ip_internal, and is_ip_internal is called repeatedly when
classifying resolved IPs (e.g., in classify_ips_allow_global_ipv6 and other flows). Moving CIDR
parsing to module scope avoids repeating the same parsing work for each call.

dist/helpers.js[238-249]
dist/helpers.js[264-276]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`is_ip_internal()` recreates and parses the same static IPv6 CIDRs on every call.

### Issue Context
This function can be called multiple times per URL validation (e.g., for each resolved IP). Even if the savings are small, it’s easy to eliminate repeated parsing.

### Fix Focus Areas
- dist/helpers.js[238-249]
- (after porting logic) src/helpers.ts[is_ip_internal implementation]

### What to do
1. Define a module-level constant like `const EXTRA_BAD_RANGES = [ipaddr.parseCIDR(...), ...]`.
2. Reuse it inside `is_ip_internal()` instead of parsing each time.
3. If `dist/` is generated, make the change in `src/` and regenerate `dist/`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@HackingRepo HackingRepo merged commit 3b1236e into main Apr 27, 2026
9 of 13 checks passed
@qodo-code-review
Copy link
Copy Markdown

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: build (24.x)

Failed stage: Run npm run build --if-present [❌]

Failed test name: ""

Failure summary:

The action failed during the npm run build --if-present step because tsc (TypeScript compiler)
reported configuration errors in tsconfig.json:
- tsconfig.json(5,25) TS5107:
compilerOptions.moduleResolution=node10 is deprecated and is treated as an error in the TypeScript
version used in CI; it requires either updating the setting or adding
compilerOptions.ignoreDeprecations: "6.0" to silence it.
- tsconfig.json(8,5) TS5011: TypeScript
detected the common source directory as ./src, but compilerOptions.rootDir is not explicitly set;
rootDir must be set (likely to ./src) to match the project layout and output structure.
These
TypeScript errors caused tsc to exit with code 2, failing the workflow.

Relevant error logs:
1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

137:  shell: /usr/bin/bash -e {0}
138:  ##[endgroup]
139:  added 108 packages, and audited 109 packages in 2s
140:  41 packages are looking for funding
141:  run `npm fund` for details
142:  1 moderate severity vulnerability
143:  To address all issues, run:
144:  npm audit fix
145:  Run `npm audit` for details.
146:  ##[group]Run npm run build --if-present
147:  �[36;1mnpm run build --if-present�[0m
148:  shell: /usr/bin/bash -e {0}
149:  ##[endgroup]
150:  > [email protected] build
151:  > tsc
152:  ##[error]tsconfig.json(5,25): error TS5107: Option 'moduleResolution=node10' is deprecated and will stop functioning in TypeScript 7.0. Specify compilerOption '"ignoreDeprecations": "6.0"' to silence this error.
153:  Visit https://aka.ms/ts6 for migration information.
154:  ##[error]tsconfig.json(8,5): error TS5011: The common source directory of 'tsconfig.json' is './src'. The 'rootDir' setting must be explicitly set to this or another path to adjust your output's file layout.
155:  Visit https://aka.ms/ts6 for migration information.
156:  ##[error]Process completed with exit code 2.
157:  Post job cleanup.

@codacy-production
Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Comment thread dist/helpers.js
Comment on lines +224 to +249
if (range !== "unicast") {
// Check for IPv4-mapped or IPv4-compatible addresses
if (range === "ipv4Mapped" || range === "rfc6145") {
try {
// @ts-ignore
const ipv4 = parsed.toIPv4Address();
return is_ip_internal(ipv4.toString());
}
catch {
return true;
}
}
return true;
}
// Additional manual blocks for IPv6
const extraBadRanges = [
ipaddr.parseCIDR("64:ff9b:1::/48"),
ipaddr.parseCIDR("5f00::/8"),
ipaddr.parseCIDR("3fff::/20"),
ipaddr.parseCIDR("fec0::/10"),
];
for (const [range, bits] of extraBadRanges) {
// @ts-ignore
if (parsed.match(range, bits))
return true;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Dist/source logic diverges 🐞 Bug ⛨ Security

The IPv6/URL hostname hardening was applied only in dist/helpers.js, while src/helpers.ts still
contains the old logic; running tsc (the project build) will overwrite dist and drop the fix. Code
paths importing from src/ (e.g., benches) will also continue using the old behavior, undermining
the intended GHSA fix.
Agent Prompt
### Issue description
Security-related IPv6/hostname validation changes were made directly in the generated file `dist/helpers.js` only. The authoritative source `src/helpers.ts` still has the previous behavior, so `npm run build` (`tsc`) will overwrite the fix and internal repo imports from `src/` will remain vulnerable.

### Issue Context
- `tsconfig.json` outputs to `dist` and includes only `src`.
- `package.json` build script is `tsc`.
- Benchmarks import from `../src/utils`.

### Fix Focus Areas
- src/helpers.ts[224-243]
- src/helpers.ts[482-520]
- dist/helpers.js[215-252]
- dist/helpers.js[425-460]

### What to do
1. Implement the same IPv6 validation logic (including IPv4-mapped handling + extra blocked ranges) in `src/helpers.ts:is_ip_internal`.
2. Apply the same hostname normalization (`parsed.hostname.replace(...)`) in `src/helpers.ts:is_url_safe` (and debug variant if intended).
3. Re-run `npm run build` to regenerate `dist/` from `src/` and commit the generated output so `dist/` matches `src/`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant